STYLE: Replace UI with IT as index type for LevelSetNode VectorContainer - #6803
Conversation
|
Still under construction. I see now: in C++, |
5bb4e81 to
67912ce
Compare
|
This force-pushed amend aims to address CI failures at https://open.cdash.org/tests/2723817696 saying: |
|
|
||
| /** Type of container used to store the level set nodes. */ | ||
| using NodeContainerType = VectorContainer<unsigned int, NodeType>; | ||
| using NodeContainerType = VectorContainer<NodeType>; |
There was a problem hiding this comment.
This is a potentially breaking change, although in practice, VectorContainer<unsigned int, NodeType> and VectorContainer<NodeType> behave exactly the same.
089ed05 to
4c1b3c9
Compare
`itk.IT` ("IdentifierType") is the most commonly supported index type for
VectorContainer.
Adjusted Node Container types in C++ to use the default index type of
`itk::VectorContainer` (which is equal to `IdentifierType`), instead of
`unsigned int`.
Adjusted ITKFastMarchingBase.wrap accordingly.
Follow-up to pull request InsightSoftwareConsortium#6800
commit 0939d4c
"STYLE: Replace UL with IT as index type in itkPyVectorContainerTest.py"
4c1b3c9 to
57480cd
Compare
|
thewtex
left a comment
There was a problem hiding this comment.
OK, but not terribly excited about this change -- I think unsigned int was probably intended, and there is a breaking type change.
Thanks for your comment, @thewtex Looking at the code, I don't see why it was using For end-users I think it would be clearer if we just stick with one index type, for |
|
I swept this PR through the ITK forest testbed (293 checked-out consumer trees). One downstream break, in ITKSphinxExamples; nothing in the C++ forest is affected. Fix opened as InsightSoftwareConsortium/ITKSphinxExamples#476.
seeds = itk.VectorContainer[itk.UI, itk.LevelSetNode[InputPixelType, Dimension]].New()That instantiation is no longer wrapped after this PR, so the example raises C++ forest: clear — every consumer reaches the container through the filter's own typedef
No Since the blast radius is source-level and confined to code that re-spells the template arguments, the ABI-only consumers just recompile. Why the forest did not catch this on its ownTwo independent gaps, both worth noting beyond this PR:
I am fixing (1) separately; the forest is currently a strong detector of C++/link breakage and a weak one of Python-wrapping breakage, and this PR is purely the latter. One minor thing on the PR itself: loop counters feeding |
itk.IT("IdentifierType") is the most commonly supported index type for VectorContainer. (VectorContainer is wrapped for IT and UC, but not for UI, as index type.)